Reviewable status: 0 of 13 files reviewed, 8 unresolved discussions (waiting on @sabercodic)

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/state/VpnSettingsUiState.kt line 15 at r2 (raw file):

    val mtu: String,
    val isAutoConnectEnabled: Boolean,
    val isConnectOnBootEnabled: Boolean?,

Just for clarification, when is it null?

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/util/BootCompletedReceiver.kt line 12 at r2 (raw file):

private const val IS_CONNECT_ON_BOOT_ENABLED_KEY = "is_connect_on_boot_enabled"
private const val TAG = "AAAAAAAAAAAABootBroadCast"

Should be removed?

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/util/BootCompletedReceiver.kt line 16 at r2 (raw file):

class BootCompletedReceiver : BroadcastReceiver() {

    override fun onReceive(context: Context?, mBootIntent: Intent?) {

This is from Java land I believe, is it possible to check if we can assume Context not to be null, or inject application context in some way? Kind of weird that we silently don't do anything if we don't have a context.

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/util/BootCompletedReceiver.kt line 28 at r2 (raw file):

                } else {

                    Log.d(TAG, "AAAAA @Boot  isConnectOnBootEnabled is false:")

Should probably clean up these logs/comments?

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/util/BootCompletedReceiver.kt line 40 at r2 (raw file):

    }

    private fun toggleTunnel(context: Context) {

toggle seems like the wrong name because the action is KEY_CONNECT_ACTION

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/util/BootCompletedReceiver.kt line 47 at r2 (raw file):

            }

        // Always start as foreground in case tile is out-of-sync.

Is this comment correct or copy-pasted from MullvadTileService? How are they connect?

android/app/src/main/kotlin/net/mullvad/mullvadvpn/di/UiModule.kt line 98 at r2 (raw file):

        SettingsRepository(
            get(),
            androidContext().getSharedPreferences(APP_PREFERENCES_NAME, Context.MODE_PRIVATE),

See we do the same thing above, shouldn't it be possible to create one instance of shared prefs and then directly inject that one?

android/app/src/main/kotlin/net/mullvad/mullvadvpn/repository/SettingsRepository.kt line 87 at r2 (raw file):

    fun setConnectOnBoot(isEnabled: Boolean) {
        sharedPreferences.edit().putBoolean(IS_CONNECT_ON_BOOT_ENABLED_KEY, isEnabled).apply()
        isConnectOnBootEnabled.value = isEnabled

This duplicate storage of value is worth discussing internally. Kind of weird that we don't have a single source of truth. E.g with DataStore we could directly get a flow of the latest value.

Would it be possible to use DataStore instead of SharedPreferences? I have used it in a hackney project and it just feels way better. I can show you my implementation if you want.

Reviewed 12 of 12 files at r1, 6 of 6 files at r2, all commit messages.
Reviewable status: all files reviewed, 8 unresolved discussions (waiting on @sabercodic)

sure

Reviewable status: all files reviewed, 8 unresolved discussions (waiting on @Rawa)

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/state/VpnSettingsUiState.kt line 15 at r2 (raw file):

On all the devices except for TVs

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/util/BootCompletedReceiver.kt line 12 at r2 (raw file):

All will remove before converting to actual PR

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/util/BootCompletedReceiver.kt line 28 at r2 (raw file):

same as above

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/util/BootCompletedReceiver.kt line 40 at r2 (raw file):

fixed

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/util/BootCompletedReceiver.kt line 47 at r2 (raw file):

Removed

android/app/src/main/kotlin/net/mullvad/mullvadvpn/repository/SettingsRepository.kt line 87 at r2 (raw file):

Fixed

Reviewed 2 of 12 files at r1, all commit messages.
Reviewable status: 11 of 13 files reviewed, 11 unresolved discussions (waiting on @Pururun, @Rawa, and @sabercodic)

android/app/src/main/AndroidManifest.xml line 101 at r3 (raw file):

                       android:resource="@xml/provider_paths" />
        </provider>
        <receiver android:name="net.mullvad.mullvadvpn.compose.util.BootCompletedReceiver"

The boot receiver is not compose specific so it should be placed in another package.

Code quote:

compose

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/VpnSettingsScreen.kt line 147 at r3 (raw file):

    onCancelMtuDialogClick: () -> Unit = {},
    onToggleAutoConnect: (Boolean) -> Unit = {},
    onToggleConnectOnBoot: (Boolean) -> Unit = {},

We probably need to discuss how these two interact and work together. For example, will "connect on boot" require the daemon "auto-connect" setting to be enabled?

Code quote:

    onToggleAutoConnect: (Boolean) -> Unit = {},
    onToggleConnectOnBoot: (Boolean) -> Unit = {},

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/state/VpnSettingsUiState.kt line 15 at r2 (raw file):

Would be nice to wrap it in some well-defined sealed class etc to make that clear

android/app/src/main/kotlin/net/mullvad/mullvadvpn/repository/SettingsRepository.kt line 33 at r3 (raw file):

    private val serviceConnectionManager: ServiceConnectionManager,
    private val sharedPreferences: SharedPreferences,
    packageManager: PackageManager,

TheSettingsRepository has hisotrically be used specifically for daemon settings, so we should probably not add SharedPreferences and PackageManager here.

Code quote:

    private val sharedPreferences: SharedPreferences,
    packageManager: PackageManager,